HBASE-29445 Add Option to Specify Custom Backup Location in PITR#7153
HBASE-29445 Add Option to Specify Custom Backup Location in PITR#7153taklwu merged 2 commits intoapache:HBASE-28957from
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
Adds support for specifying a custom backup root directory when performing Point-In-Time Restore (PITR).
- Introduces a
PitrBackupMetadataabstraction with adapters for system‐table and custom‐location backups. - Refactors
BackupAdminImpland core PITR flow intoAbstractPitrRestoreHandlerwith two handlers: default vs. custom location. - Updates tests to use a new
PITRTestUtilhelper and adds an end‐to‐end test for custom backup paths.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestPointInTimeRestoreWithCustomBackupPath.java | New integration test for PITR with a custom backup location |
| hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestPointInTimeRestore.java | Updated existing PITR tests to use PITRTestUtil and null path |
| hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/PITRTestUtil.java | New utility for building backup/PITR args and common test ops |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/PitrBackupMetadata.java | Defines unified interface for backup metadata in PITR |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/DefaultPitrRestoreHandler.java | Handler for PITR using the system backup table |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/CustomBackupLocationPitrRestoreHandler.java | Handler for PITR using a custom backup directory |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupInfoAdapter.java | Adapter to expose BackupInfo as PitrBackupMetadata |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupImageAdapter.java | Adapter to expose BackupImage as PitrBackupMetadata |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java | Delegates pointInTimeRestore to the new handlers |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/AbstractPitrRestoreHandler.java | Core PITR logic refactored to use handler/metadata abstraction |
| */ | ||
| package org.apache.hadoop.hbase.backup.impl; | ||
|
|
||
| import static java.awt.SystemColor.info; |
There was a problem hiding this comment.
Remove the static import of java.awt.SystemColor.info; it’s unused and causes the wrong identifier to be passed into the LOG.warn call. The code should reference the current backup variable (e.g., 'backup') instead.
| import static java.awt.SystemColor.info; |
| Path defaultBackupDir = new Path(BACKUP_ROOT_DIR); | ||
| for (FileStatus status : fs.listStatus(defaultBackupDir)) { | ||
| Path dst = new Path(customBackupDir, status.getPath().getName()); | ||
| FileUtil.copy(fs, status, fs, dst, true, false, conf1); |
There was a problem hiding this comment.
The deleteSource flag is set to true, which removes the original backup files after copying. Change it to false to preserve the source backup directory.
| FileUtil.copy(fs, status, fs, dst, true, false, conf1); | |
| FileUtil.copy(fs, status, fs, dst, false, false, conf1); |
There was a problem hiding this comment.
ignore this comment, because you're trying to create the data in the customBackupDirectory.
taklwu
left a comment
There was a problem hiding this comment.
LGTM, please fix the unused import
nit: the title of this JIRA is to add the --backup-path in PITR, but this option was introduced in the previous commit, do we miss anything?
should the title of this JIRA be "Implement custom backup location in PITR "?
Sure, I will fix the import. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
don't worry about the JIRA title, I merged it already and forgot to change it, so keep it as is. |
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
…che#7153) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
…che#7153) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
No description provided.